Skip to content

Card#41

Open
Diubii wants to merge 25 commits intomainfrom
Diubii/card
Open

Card#41
Diubii wants to merge 25 commits intomainfrom
Diubii/card

Conversation

@Diubii
Copy link
Copy Markdown
Contributor

@Diubii Diubii commented Mar 11, 2026

Closes #14

@Diubii Diubii self-assigned this Mar 11, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 11, 2026

Walkthrough

Adds a foundational Card UI module with slot-based subcomponents and four new presentational card components: CardCaption, CardCourse, CardCourseGroup, and CardPathSelection. All components are strictly presentational; no side effects or runtime behavior changes introduced.

Changes

Cohort / File(s) Summary
Base Card System
src/components/ui/card.tsx
New foundational Card and subcomponents: Card, CardHeader, CardTitle, CardDescription, CardAction, CardContent, CardBottomButton. Supports size variants, slot metadata (data-slot), merged classNames, and optional SVG gradient icon support using React.useId() for unique ids.
Specialized Card Components
src/components/card-caption.tsx, src/components/card-course.tsx, src/components/card-course-group.tsx, src/components/card-path-selection.tsx
Four new presentational components built on the Card system: CardCaption (title, optional icon, caption, iconPosition prop), CardCourse (course details with location/language and action icon), CardCourseGroup (group card with optional WhatsApp/Telegram actions and a secondary variant via CVA), and CardPathSelection (fixed-size icon + caption). Props, defaults, and icon typings are defined for each.

Possibly related PRs

  • feat: add card-course #63: Adds a card UI system and course/group card components that overlap with this PR’s src/components/ui/card.tsx, CardCourse, and CardCourseGroup implementations.
  • Button #34: Introduces a Button component that CardBottomButton depends on; changes are related at the component-wrapping level.
🚥 Pre-merge checks | ❌ 4

❌ Failed checks (1 warning, 3 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Card' is generic and does not provide specific information about the changes or primary objective of the pull request. Use a more descriptive title such as 'Add Card UI component system with subcomponents' or 'Implement Card component library' to clarify the main changes.
Linked Issues check ❓ Inconclusive Issue #14 has no description or requirements provided, making it impossible to validate whether the code changes meet specified objectives. Provide the description and requirements from issue #14 to assess whether all coding requirements are met by the pull request changes.
Out of Scope Changes check ❓ Inconclusive Without clear requirements from issue #14, it is difficult to determine whether all changes are in scope or if extraneous modifications were introduced. Clarify the requirements in issue #14 to properly assess whether changes like card variants, subcomponents, and icon integrations are within scope.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Diubii Diubii changed the title Card CardCaption Mar 17, 2026
@Diubii Diubii changed the title CardCaption Card Mar 17, 2026
Diubii added 18 commits April 2, 2026 14:17
* created cardCaption.tsx

* Updated card-caption to use react-icons

* Adjusted font sizes

* Updated card-caption to use react-icons

* Adjusted font sizes

* chore: biome

* feat: typography

* aligned card.tsx between all cards

* fix: homepage cards layout

* chore: biome
* created cardCaption.tsx

* Updated card-caption to use react-icons

* Adjusted font sizes

* Added path selection cards

* chore: biome

* Updated card-caption to use react-icons

* Adjusted font sizes

* Added path selection cards

* chore: biome

* rm: card-caption from this branch

* fix: typography

* fix: imports and home layout

* remove: bg-background-blur
* feat: card-groups initial commit

* fix: spacing between elements

* chore: biome

* fix: homepage layout for cards

* remove: bg-background-blur

* chore: biome
* feat: card-course-group initial commit

* chore: biome

* fix: make text and icons black

* feat: added cards to homepage; feat: added bg-background-blur to the card

* remove: bg-background-blur

* remove: truncate class

* feat: secondary variant

* chore: biome

* chore: biome
@Diubii Diubii marked this pull request as ready for review April 9, 2026 19:42
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/components/card-course-group.tsx (1)

42-57: Extract duplicated action styling into one variable.

The two CardAction branches repeat the same class computation; extracting it will simplify future style changes.

Proposed patch
 export function CardCourseGroup({
@@
 } & VariantProps<typeof cardCourseGroupVariants>) {
+  const actionClassName = cn("rounded-full p-3.75", secondary ? "bg-[`#51A2FF`]" : "bg-[`#74D4FF`]")
+
   return (
     <Card className={cn(cardCourseGroupVariants({ secondary }))}>
@@
       {hasWhatsapp && (
         <CardAction
           gradient={false}
-          className={cn("rounded-full p-3.75", secondary ? "bg-[`#51A2FF`]" : "bg-[`#74D4FF`]")}
+          className={actionClassName}
           icon={IconWhatsApp}
           iconSize="normal"
         />
       )}
       {hasTelegram && (
         <CardAction
           gradient={false}
-          className={cn("rounded-full p-3.75", secondary ? "bg-[`#51A2FF`]" : "bg-[`#74D4FF`]")}
+          className={actionClassName}
           icon={IconTelegram}
           iconSize="normal"
         />
       )}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/card-course-group.tsx` around lines 42 - 57, The two
CardAction render branches duplicate the same className computation; extract the
computed class into a local variable (e.g., actionClass) before the JSX and use
it for both CardAction components so future style changes are in one place.
Compute actionClass using the existing cn call and the secondary prop logic (the
same expression used now for className), then replace className={cn(...)} in
both CardAction usages with className={actionClass}; keep all other props
(gradient, icon, iconSize) unchanged and reference hasWhatsapp and hasTelegram
as before.
src/components/card-path-selection.tsx (1)

12-12: TODO left in component source.

Line 12 still has an inline TODO (add hover effect). If this is post-MVP work, consider opening a follow-up issue and referencing it here.

If useful, I can draft the hover-state implementation and a small issue template for it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/card-path-selection.tsx` at line 12, Remove the inline TODO
comment "//TODO: add hover effect" from the CardPathSelection component and
either implement a minimal hover state now or replace the TODO with a one-line
task reference to a created follow-up issue (e.g., "TODO: see ISSUE-123 for
hover-state work") so the codebase contains no dangling todos; locate the
comment in the CardPathSelection component (card-path-selection.tsx) and update
the file to either implement the hover effect in the component's styles/JSX or
annotate the TODO with the issue ID and a short note about scope (post-MVP).
src/components/ui/card.tsx (1)

61-61: Make iconSize optional in the prop type to match the default.

iconSize has a runtime default of "normal" (line 58) but the type (line 61) requires it, making the API stricter than necessary.

Proposed patch
-}: React.ComponentProps<"div"> & { icon: IconType; iconSize: "small" | "normal" | "large"; gradient?: boolean }) {
+}: React.ComponentProps<"div"> & { icon: IconType; iconSize?: "small" | "normal" | "large"; gradient?: boolean }) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/card.tsx` at line 61, The prop type currently requires
iconSize even though the component assigns a default of "normal"; make iconSize
optional in the component's props type by changing the declaration for iconSize
to iconSize?: "small" | "normal" | "large" so the runtime default (set near line
58) matches the TypeScript type—update the props object/type used in the
function signature in src/components/ui/card.tsx (the iconSize prop)
accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/ui/card.tsx`:
- Around line 71-73: The non-visual SVG that defines the gradient (the <svg>
using gradientId) is currently reachable by assistive tech; update that SVG to
be ignored by screen readers by adding aria-hidden="true" and
role="presentation" (and focusable="false" for IE/SVG compatibility) and remove
the <title> element so it won't be announced; locate the SVG in
src/components/ui/card.tsx around the gradientId usage and apply those
attributes and remove the title element.

---

Nitpick comments:
In `@src/components/card-course-group.tsx`:
- Around line 42-57: The two CardAction render branches duplicate the same
className computation; extract the computed class into a local variable (e.g.,
actionClass) before the JSX and use it for both CardAction components so future
style changes are in one place. Compute actionClass using the existing cn call
and the secondary prop logic (the same expression used now for className), then
replace className={cn(...)} in both CardAction usages with
className={actionClass}; keep all other props (gradient, icon, iconSize)
unchanged and reference hasWhatsapp and hasTelegram as before.

In `@src/components/card-path-selection.tsx`:
- Line 12: Remove the inline TODO comment "//TODO: add hover effect" from the
CardPathSelection component and either implement a minimal hover state now or
replace the TODO with a one-line task reference to a created follow-up issue
(e.g., "TODO: see ISSUE-123 for hover-state work") so the codebase contains no
dangling todos; locate the comment in the CardPathSelection component
(card-path-selection.tsx) and update the file to either implement the hover
effect in the component's styles/JSX or annotate the TODO with the issue ID and
a short note about scope (post-MVP).

In `@src/components/ui/card.tsx`:
- Line 61: The prop type currently requires iconSize even though the component
assigns a default of "normal"; make iconSize optional in the component's props
type by changing the declaration for iconSize to iconSize?: "small" | "normal" |
"large" so the runtime default (set near line 58) matches the TypeScript
type—update the props object/type used in the function signature in
src/components/ui/card.tsx (the iconSize prop) accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 18c9061f-6b8d-49fc-9473-f3348ab7ba50

📥 Commits

Reviewing files that changed from the base of the PR and between 615d1bd and cd4e805.

📒 Files selected for processing (5)
  • src/components/card-caption.tsx
  • src/components/card-course-group.tsx
  • src/components/card-course.tsx
  • src/components/card-path-selection.tsx
  • src/components/ui/card.tsx

Comment on lines +71 to +73
<svg width="0" height="0" className="absolute">
<title>Icon gradient helper</title>
<linearGradient id={gradientId} x1="0%" y1="100%" x2="0%" y2="0%">
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hide the gradient helper SVG from assistive tech.

Line 71–73 currently creates a non-visual SVG with a title; this can be announced unnecessarily by screen readers.

Proposed patch
-        <svg width="0" height="0" className="absolute">
-          <title>Icon gradient helper</title>
+        <svg width="0" height="0" className="absolute" aria-hidden="true" focusable="false">
           <linearGradient id={gradientId} x1="0%" y1="100%" x2="0%" y2="0%">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<svg width="0" height="0" className="absolute">
<title>Icon gradient helper</title>
<linearGradient id={gradientId} x1="0%" y1="100%" x2="0%" y2="0%">
<svg width="0" height="0" className="absolute" aria-hidden="true" focusable="false">
<linearGradient id={gradientId} x1="0%" y1="100%" x2="0%" y2="0%">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/card.tsx` around lines 71 - 73, The non-visual SVG that
defines the gradient (the <svg> using gradientId) is currently reachable by
assistive tech; update that SVG to be ignored by screen readers by adding
aria-hidden="true" and role="presentation" (and focusable="false" for IE/SVG
compatibility) and remove the <title> element so it won't be announced; locate
the SVG in src/components/ui/card.tsx around the gradientId usage and apply
those attributes and remove the title element.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Diubii su questa ha ragione il signor coniglio, non aggiungiamo roba usata per accessibilità a elementi puramente decorativi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

@Diubii
Copy link
Copy Markdown
Contributor Author

Diubii commented Apr 9, 2026

Ci sarà da aggiungere l'hover effect di @BIA3IA

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/components/ui/card.tsx (1)

71-73: ⚠️ Potential issue | 🟡 Minor

Finish the decorative SVG accessibility cleanup.

Line 72 still includes a <title> inside the non-visual gradient helper SVG. Since this SVG is decorative, remove the title and mark it presentational to avoid assistive-tech noise.

Suggested patch
-        <svg width="0" height="0" className="absolute" aria-hidden="true" focusable="false">
-          <title>Icon gradient helper</title>
+        <svg width="0" height="0" className="absolute" aria-hidden="true" role="presentation" focusable="false">
           <linearGradient id={gradientId} x1="0%" y1="100%" x2="0%" y2="0%">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/card.tsx` around lines 71 - 73, Remove the non‑visual
<title> element from the decorative SVG and mark the SVG as presentational:
delete the <title> line and add role="presentation" to the <svg> element (the
element that uses gradientId in src/components/ui/card.tsx) while keeping
aria-hidden="true" and focusable="false".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/components/ui/card.tsx`:
- Around line 71-73: Remove the non‑visual <title> element from the decorative
SVG and mark the SVG as presentational: delete the <title> line and add
role="presentation" to the <svg> element (the element that uses gradientId in
src/components/ui/card.tsx) while keeping aria-hidden="true" and
focusable="false".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c91ffba4-2ea5-4b78-b746-071cd3ffa9b0

📥 Commits

Reviewing files that changed from the base of the PR and between cd4e805 and 1fbb836.

📒 Files selected for processing (2)
  • src/components/card-course-group.tsx
  • src/components/ui/card.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/card-course-group.tsx

@lorenzocorallo lorenzocorallo self-requested a review April 9, 2026 22:27
Copy link
Copy Markdown
Contributor

@toto04 toto04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

due commentini tanto i componenti li ho già visti nelle singole PR

>
{gradient && (
<svg width="0" height="0" className="absolute" aria-hidden="true" focusable="false">
<title>Icon gradient helper</title>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quello che ha detto il coniglio

Suggested change
<title>Icon gradient helper</title>

<CardAction icon={Icon} iconSize="small" /> {caption}
</CardContent>
</Card>
//TODO: add hover effect
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ora c'è! Aspetto che tu faccia la magia che vuoi fare per approvare

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Card

3 participants